[FEATURE] Update cachemanager and cacherepository implementations to async#715
Conversation
Reviewer's GuideThis PR overhauls the caching layer by migrating all CacheManager methods, repository interfaces, and implementations to asynchronous APIs with CancellationToken support, built-in default timeouts, enhanced logging and error handling, and automatic promotion of fetched entries into the in-memory cache. Tests have been updated to exercise the new async methods. Sequence diagram for async GetAsync with automatic memory cache promotionsequenceDiagram
participant Client
participant CacheManager
participant MemoryCacheRepository
participant RedisCacheRepository
participant CouchDBCacheRepository
Client->>CacheManager: GetAsync<T>(key, cancellationToken)
CacheManager->>MemoryCacheRepository: GetAsync<T>(key, cancellationToken)
alt Found in memory
MemoryCacheRepository-->>CacheManager: value
CacheManager-->>Client: value
else Not found in memory
CacheManager->>RedisCacheRepository: GetAsync<T>(key, cancellationToken)
alt Found in Redis
RedisCacheRepository-->>CacheManager: value
CacheManager->>MemoryCacheRepository: SetAsync<T>(value, key, CancellationToken.None)
CacheManager-->>Client: value
else Not found in Redis
CacheManager->>CouchDBCacheRepository: GetAsync<T>(key, cancellationToken)
alt Found in CouchDB
CouchDBCacheRepository-->>CacheManager: value
CacheManager->>MemoryCacheRepository: SetAsync<T>(value, key, CancellationToken.None)
CacheManager-->>Client: value
else Not found in any
CouchDBCacheRepository-->>CacheManager: (throws)
CacheManager-->>Client: (throws InvalidOperationException)
end
end
end
Class diagram for updated ICacheRepository interface and implementationsclassDiagram
class ICacheRepository {
<<interface>>
+ValueTask SetAsync<T>(T value, string key, TimeSpan? ttl = null, CancellationToken cancellationToken = default)
+ValueTask SetAsync<T>(T value, string key, string subKey, CancellationToken cancellationToken = default)
+ValueTask<T> GetAsync<T>(string key, CancellationToken cancellationToken = default)
+ValueTask<T> GetAsync<T>(string key, string subKey, CancellationToken cancellationToken = default)
+ValueTask<(bool Success, T Value)> TryGetAsync<T>(string key, CancellationToken cancellationToken = default)
+ValueTask<(bool Success, T Value)> TryGetAsync<T>(string key, string subKey, CancellationToken cancellationToken = default)
+Task RemoveAsync(string key, CancellationToken cancellationToken = default)
+Task RemoveAsync(string key, string subKey, CancellationToken cancellationToken = default)
+Task<TimeSpan> TTLAsync(string key, CancellationToken cancellationToken = default)
+Task Clear()
}
class MemoryCacheRepository {
+ValueTask SetAsync<T>(T value, string key, TimeSpan? ttl = null, CancellationToken cancellationToken = default)
+ValueTask SetAsync<T>(T value, string key, string subKey, CancellationToken cancellationToken = default)
+ValueTask<T> GetAsync<T>(string key, CancellationToken cancellationToken = default)
+ValueTask<T> GetAsync<T>(string key, string subKey, CancellationToken cancellationToken = default)
+ValueTask<(bool Success, T Value)> TryGetAsync<T>(string key, CancellationToken cancellationToken = default)
+ValueTask<(bool Success, T Value)> TryGetAsync<T>(string key, string subKey, CancellationToken cancellationToken = default)
+Task RemoveAsync(string key, CancellationToken cancellationToken = default)
+Task RemoveAsync(string key, string subKey, CancellationToken cancellationToken = default)
+Task<TimeSpan> TTLAsync(string key, CancellationToken cancellationToken = default)
+Task Clear()
}
class RedisCacheRepository {
+ValueTask SetAsync<T>(T value, string key, TimeSpan? ttl = null, CancellationToken cancellationToken = default)
+ValueTask SetAsync<T>(T value, string key, string subKey, CancellationToken cancellationToken = default)
+ValueTask<T> GetAsync<T>(string key, CancellationToken cancellationToken = default)
+ValueTask<T> GetAsync<T>(string key, string subKey, CancellationToken cancellationToken = default)
+ValueTask<(bool Success, T Value)> TryGetAsync<T>(string key, CancellationToken cancellationToken = default)
+ValueTask<(bool Success, T Value)> TryGetAsync<T>(string key, string subKey, CancellationToken cancellationToken = default)
+Task RemoveAsync(string key, CancellationToken cancellationToken = default)
+Task RemoveAsync(string key, string subKey, CancellationToken cancellationToken = default)
+Task<TimeSpan> TTLAsync(string key, CancellationToken cancellationToken = default)
+Task Clear()
}
class CouchDBCacheRepository {
+ValueTask SetAsync<T>(T value, string key, TimeSpan? ttl = null, CancellationToken cancellationToken = default)
+ValueTask SetAsync<T>(T value, string key, string subKey, CancellationToken cancellationToken = default)
+ValueTask<T> GetAsync<T>(string key, CancellationToken cancellationToken = default)
+ValueTask<T> GetAsync<T>(string key, string subKey, CancellationToken cancellationToken = default)
+ValueTask<(bool Success, T Value)> TryGetAsync<T>(string key, CancellationToken cancellationToken = default)
+ValueTask<(bool Success, T Value)> TryGetAsync<T>(string key, string subKey, CancellationToken cancellationToken = default)
+Task RemoveAsync(string key, CancellationToken cancellationToken = default)
+Task RemoveAsync(string key, string subKey, CancellationToken cancellationToken = default)
+Task<TimeSpan> TTLAsync(string key, CancellationToken cancellationToken = default)
+Task Clear()
}
ICacheRepository <|.. MemoryCacheRepository
ICacheRepository <|.. RedisCacheRepository
ICacheRepository <|.. CouchDBCacheRepository
Class diagram for updated CacheManager async APIclassDiagram
class CacheManager {
<<static>>
+ValueTask SetAsync<T>(T value, string key, CancellationToken cancellationToken = default)
+ValueTask SetAsync<T>(T value, string key, string subKey, CancellationToken cancellationToken = default)
+ValueTask SetAsync<T>(T value, string key, TimeSpan ttl, CancellationToken cancellationToken = default)
+ValueTask SetToAsync<TCacheRepository, TValue>(TValue value, string key, CancellationToken cancellationToken = default)
+ValueTask SetToAsync<TCacheRepository, TValue>(TValue value, string key, string subKey, CancellationToken cancellationToken = default)
+ValueTask SetToAsync<TCacheRepository, TValue>(TValue value, string key, TimeSpan ttl, CancellationToken cancellationToken = default)
+ValueTask SetToAsync<TCacheRepository, TValue>(TValue value, string key, string subKey, TimeSpan ttl, CancellationToken cancellationToken = default)
+ValueTask<T> GetAsync<T>(string key, CancellationToken cancellationToken = default)
+ValueTask<T> GetAsync<T>(string key, string subKey, CancellationToken cancellationToken = default)
+ValueTask<TValue> GetFrom<TCacheRepository, TValue>(string key, CancellationToken cancellationToken = default)
+ValueTask<TValue> GetFromAsync<TCacheRepository, TValue>(string key, string subKey, CancellationToken cancellationToken = default)
+ValueTask<(bool Success, T Value)> TryGetAsync<T>(string key, CancellationToken cancellationToken = default)
+ValueTask<bool> TryGetAsync<T>(string key, string subKey, CancellationToken cancellationToken = default)
+Task<TimeSpan> TTLAsync(string key)
+ValueTask Remove(string key, CancellationToken cancellationToken = default)
+ValueTask Remove(string key, string subKey, CancellationToken cancellationToken = default)
+ValueTask RemoveFrom<TCacheRepository>(string key, CancellationToken cancellationToken = default)
+ValueTask RemoveFrom<TCacheRepository>(string key, string subKey, CancellationToken cancellationToken = default)
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment
|
![gstraccini[bot]](https://avatars.githubusercontent.com/in/480132?s=60&v=4){kind=small}
gstraccini
Bot
added
the
🚦 awaiting triage
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
WalkthroughThis change set updates cache manager tests to use asynchronous cache APIs and async exception assertions, and adds a VS Code setting for Snyk organization selection. ChangesAsync cache manager tests
Editor settings
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi I request an early feedback before I update Test cases. |
|
❌ Build CrispyWaffle 10.0.352 failed (commit 3e9349bd33 by @HarryKambo) |
…ispyWaffle into feature/cacheasync hanges from master branch merged
|
✅ Build CrispyWaffle 10.0.354 completed (commit 1f9fc7b543 by @code-factor) |
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sonarcsharp (reported by Codacy) found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
|
Hi @HarryKambo 👋, Thank you so much for your pull request! 🙌 I appreciate the time and effort you put into this contribution. Thanks again for your valuable contribution! 🚀 |
|
Hi @guibranco |
|
✅ Build CrispyWaffle 10.0.370 completed (commit 8985d38231 by @guibranco) |
guibranco
left a comment
guibranco
left a comment
There was a problem hiding this comment.
Hi @HarryKambo 👋, this PR looks good to me, do you plan to do more work over it? Or it's ready to approve and merge?
guibranco
added
📝 documentation
guibranco
changed the title
cachemanager and cacherepository implementations to async
|
✅ Build CrispyWaffle 10.0.1545 completed (commit 5bb80ee8c5 by @guibranco) |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 13768420 | Triggered | Generic Password | 6f5e4e1 | .github/workflows/build.yml | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
@gstraccini csharpier |
|
Running CSharpier on this branch! 🔧 |
|
❌ CSharpier failed! |
Modify CacheManager to use TryGetAsync for retrieving cache items, enabling operations to return success status along with the cached value. This refactoring improves error handling and ensures that exceptions only occur when explicitly thrown, such as during timeouts or cancellation. Update tests to reflect new TTL handling and remove redundant operations. Enhance settings.json to autoselect organization for Snyk, ensuring configurations are aligned for automation and security purposes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
Tests/CrispyWaffle.Tests/Cache/CacheManagerTests.cs (1)
147-163: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTest won't actually assert the exception — missing
async Task+await.The method is still
public void, andact.Should().ThrowAsync<InvalidOperationException>().WithMessage(...)returns an unawaitedTask. Since the test method itself isn't async and doesn't await this task, the assertion runs fire-and-forget after the test method returns — the test can report as passing even if the thrown exception/message doesn't match.🐛 Proposed fix
- public void RemoveFromShouldThrowWhenRepositoryNotFound() + public async Task RemoveFromShouldThrowWhenRepositoryNotFound() { // Arrange var key = "testKey"; CacheManager.AddRepository(_mockCacheRepository); // Act Func<Task> act = async () => await CacheManager.RemoveFrom<ICacheRepository>(key); // Assert - act.Should() + await act.Should() .ThrowAsync<InvalidOperationException>() .WithMessage( "The repository of type CrispyWaffle.Cache.ICacheRepository isn't available in the repositories providers list" ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/CrispyWaffle.Tests/Cache/CacheManagerTests.cs` around lines 147 - 163, The test in CacheManagerTests.RemoveFromShouldThrowWhenRepositoryNotFound is not awaiting the async exception assertion, so it may pass without validating the thrown InvalidOperationException. Change the test method to return an async Task and await the ThrowAsync/WithMessage assertion on act so the exception and message are actually verified. Use the existing RemoveFrom<ICacheRepository> and CacheManager.AddRepository setup to keep the test behavior the same.Src/CrispyWaffle/Cache/CacheManager.cs (4)
1008-1012: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemoval failures are swallowed with no logging at all.
Unlike the equivalent catch blocks in
GetAsync/TryGetAsync(e.g. Lines 524-530), the genericcatch (Exception ex)here only appends to the localerrorslist — there is noLogConsumer.Errorcall:catch (Exception ex) { var repositoryName = repository.GetType().Name; errors.Add((repositoryName, ex)); }Since
errorsis never read after the loop, a failed removal from a repository (e.g. Redis timeout, CouchDB conflict) leaves stale data behind with zero trace in logs, and the method returns as if the removal succeeded everywhere.🐛 Proposed fix
catch (Exception ex) { var repositoryName = repository.GetType().Name; errors.Add((repositoryName, ex)); + LogConsumer.Error("Failed to remove key {0} from repository {1}: {2}", key, repositoryName, ex.Message); }Also applies to: 1068-1072
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Src/CrispyWaffle/Cache/CacheManager.cs` around lines 1008 - 1012, Removal failures in CacheManager are being swallowed silently in the repository loop, so add a LogConsumer.Error call inside the catch block in the removal path(s) while keeping the existing errors list update if needed. Use the same logging pattern as GetAsync/TryGetAsync and include the repositoryName plus exception details so failed removals are visible. Apply this fix to both removal catch blocks in CacheManager to cover all paths.
261-291: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCancellation is silently swallowed in all
SetToAsyncoverloads, contradicting the documented contract.Each overload's XML doc promises
<exception cref="OperationCanceledException">If cancelled.</exception>(Lines 259, 303, 348, 399), but the implementation catchesOperationCanceledExceptionand just logs to console without rethrowing:catch (OperationCanceledException) { Console.WriteLine("Cache operation was cancelled"); }A caller that cancels the token gets a normally-completed
ValueTaskas if the write succeeded — this can mask cancellation-driven aborts in calling code (e.g. pipelines that rely onOperationCanceledExceptionto unwind).🐛 Proposed fix (repeat for all four overloads)
catch (OperationCanceledException) { - Console.WriteLine("Cache operation was cancelled"); + LogConsumer.Trace("Cache operation was cancelled"); + throw; }Also applies to: 305-336, 350-386, 401-439
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Src/CrispyWaffle/Cache/CacheManager.cs` around lines 261 - 291, The SetToAsync overloads in CacheManager are swallowing OperationCanceledException instead of honoring the documented cancellation contract. Update each SetToAsync<TCacheRepository, TValue> overload to let cancellation propagate from repository.SetAsync by removing the special catch block or rethrowing the cancellation exception after any optional logging, and keep the behavior consistent across all overloads. Use the existing CacheManager.SetToAsync methods and the repository.SetAsync call as the main locations to apply the fix.
647-647: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd Async suffix to the remaining public cache APIs The non-suffixed async overloads (
GetFrom,Remove,RemoveFrom) break the naming pattern used elsewhere inCacheManagerand make these entry points look synchronous. Rename them toGetFromAsync,RemoveAsync, andRemoveFromAsyncto keep the public API consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Src/CrispyWaffle/Cache/CacheManager.cs` at line 647, The remaining public async cache APIs in CacheManager are missing the Async suffix, which makes them inconsistent with the rest of the public surface. Rename the async overloads GetFrom, Remove, and RemoveFrom to GetFromAsync, RemoveAsync, and RemoveFromAsync, and update any related references/usages so the public API naming stays consistent and clearly asynchronous.
401-439: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winForward
ttlin this overload.
subKeyis already passed through, butttlis discarded because the call only usesSetAsync(value, key, subKey, ...).ICacheRepositoryhas no combined(key, subKey, ttl)overload, so callers of this method will never get expiration unless a new repository API is added. The trace format also uses{2}twice, sosubKeyis omitted from the log.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Src/CrispyWaffle/Cache/CacheManager.cs` around lines 401 - 439, The SetToAsync<TCacheRepository, TValue> overload is dropping the ttl argument and the trace message is also formatting the parameters incorrectly, so expiration and subKey visibility are lost. Update CacheManager.SetToAsync to propagate ttl through to the repository call by using the appropriate repository API or adding one if needed, and fix the LogConsumer.Trace argument order so key, repository type, ttl, and subKey are all logged correctly. Keep the existing cancellation and exception handling intact.
🧹 Nitpick comments (3)
.vscode/settings.json (1)
22-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnrelated editor setting change bundled into async cache PR.
This Snyk organization setting is unrelated to the async cache manager feature described in the PR objectives. Consider moving it to a separate PR to keep the diff focused.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.vscode/settings.json around lines 22 - 26, The async cache PR includes an unrelated editor configuration change in the VS Code settings. Remove the Snyk organization setting from this change set and keep the editor-only update separate from the async cache work; use the existing settings entries in the .vscode/settings.json diff as the place to locate and split it out.Src/CrispyWaffle/Cache/CacheManager.cs (2)
466-466: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollected
errorsare never surfaced to the caller.
GetAsync(x2),TryGetAsync(x2), andRemove(x2) all build a localerrorslist during the repository loop, but it's discarded once the loop ends — the finalInvalidOperationException("Unable to get the item with key {key}")(e.g. Line 539) includes no detail about what actually failed in each repository.Consider aggregating
errorsinto the thrown exception (e.g. viaAggregateExceptionasInnerException, or appended to the message) so failures are diagnosable without re-enabling debug logging.Also applies to: 562-562, 760-760, 860-860, 981-981, 1041-1041
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Src/CrispyWaffle/Cache/CacheManager.cs` at line 466, The local errors list built in CacheManager methods like GetAsync, TryGetAsync, and Remove is discarded before the final exception is thrown, so the caller gets no repository failure details. Update the final throw sites in these methods to include the collected errors, either by wrapping them in an AggregateException/InnerException or by appending repository names and exception messages to the existing InvalidOperationException text. Use the existing errors list and the repository loop variables in CacheManager to locate each affected throw path.
941-961: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
TTLAsyncdoesn't accept aCancellationToken, unlike every other public method in this class.Given the PR's goal of adding cancellation-token support across cache operations, this method is the one outlier without it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Src/CrispyWaffle/Cache/CacheManager.cs` around lines 941 - 961, TTLAsync is the remaining public cache operation that does not accept a CancellationToken. Update the CacheManager.TTLAsync signature to take a CancellationToken, then thread it through each repository.TTLAsync call so cancellation is honored consistently with the other public methods. Keep the existing TTL aggregation logic and adjust any callers or interface implementations that reference TTLAsync.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Src/CrispyWaffle/Cache/CacheManager.cs`:
- Around line 1008-1012: Removal failures in CacheManager are being swallowed
silently in the repository loop, so add a LogConsumer.Error call inside the
catch block in the removal path(s) while keeping the existing errors list update
if needed. Use the same logging pattern as GetAsync/TryGetAsync and include the
repositoryName plus exception details so failed removals are visible. Apply this
fix to both removal catch blocks in CacheManager to cover all paths.
- Around line 261-291: The SetToAsync overloads in CacheManager are swallowing
OperationCanceledException instead of honoring the documented cancellation
contract. Update each SetToAsync<TCacheRepository, TValue> overload to let
cancellation propagate from repository.SetAsync by removing the special catch
block or rethrowing the cancellation exception after any optional logging, and
keep the behavior consistent across all overloads. Use the existing
CacheManager.SetToAsync methods and the repository.SetAsync call as the main
locations to apply the fix.
- Line 647: The remaining public async cache APIs in CacheManager are missing
the Async suffix, which makes them inconsistent with the rest of the public
surface. Rename the async overloads GetFrom, Remove, and RemoveFrom to
GetFromAsync, RemoveAsync, and RemoveFromAsync, and update any related
references/usages so the public API naming stays consistent and clearly
asynchronous.
- Around line 401-439: The SetToAsync<TCacheRepository, TValue> overload is
dropping the ttl argument and the trace message is also formatting the
parameters incorrectly, so expiration and subKey visibility are lost. Update
CacheManager.SetToAsync to propagate ttl through to the repository call by using
the appropriate repository API or adding one if needed, and fix the
LogConsumer.Trace argument order so key, repository type, ttl, and subKey are
all logged correctly. Keep the existing cancellation and exception handling
intact.
In `@Tests/CrispyWaffle.Tests/Cache/CacheManagerTests.cs`:
- Around line 147-163: The test in
CacheManagerTests.RemoveFromShouldThrowWhenRepositoryNotFound is not awaiting
the async exception assertion, so it may pass without validating the thrown
InvalidOperationException. Change the test method to return an async Task and
await the ThrowAsync/WithMessage assertion on act so the exception and message
are actually verified. Use the existing RemoveFrom<ICacheRepository> and
CacheManager.AddRepository setup to keep the test behavior the same.
---
Nitpick comments:
In @.vscode/settings.json:
- Around line 22-26: The async cache PR includes an unrelated editor
configuration change in the VS Code settings. Remove the Snyk organization
setting from this change set and keep the editor-only update separate from the
async cache work; use the existing settings entries in the .vscode/settings.json
diff as the place to locate and split it out.
In `@Src/CrispyWaffle/Cache/CacheManager.cs`:
- Line 466: The local errors list built in CacheManager methods like GetAsync,
TryGetAsync, and Remove is discarded before the final exception is thrown, so
the caller gets no repository failure details. Update the final throw sites in
these methods to include the collected errors, either by wrapping them in an
AggregateException/InnerException or by appending repository names and exception
messages to the existing InvalidOperationException text. Use the existing errors
list and the repository loop variables in CacheManager to locate each affected
throw path.
- Around line 941-961: TTLAsync is the remaining public cache operation that
does not accept a CancellationToken. Update the CacheManager.TTLAsync signature
to take a CancellationToken, then thread it through each repository.TTLAsync
call so cancellation is honored consistently with the other public methods. Keep
the existing TTL aggregation logic and adjust any callers or interface
implementations that reference TTLAsync.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 583fbaa9-f3da-48a8-be8e-97c93fbcb493
📒 Files selected for processing (3)
.vscode/settings.jsonSrc/CrispyWaffle/Cache/CacheManager.csTests/CrispyWaffle.Tests/Cache/CacheManagerTests.cs
Replace direct calls to `_testOutputHelper.WriteLine` with a new `SafeWriteLine` method. This change addresses the issue of `InvalidOperationException` being thrown when test output occurs after the test has completed. `SafeWriteLine` wraps the write operations in a try-catch block to safely ignore these exceptions, ensuring more robust handling of log output during parallel test execution.
Add conditional checks to prevent executing SonarCloud and Codecov steps on pull requests from forks. This ensures that sensitive information (such as tokens) is only used when the head repository matches the main repository, thus enhancing security by avoiding exposure on external PRs.
Change the concurrency test's task delay from async Task.Delay to Thread.Sleep. This modification improves the test reliability by ensuring precise execution timing, as Task.Delay could be subject to task scheduler variations, affecting the accuracy of concurrency measurements.
5 participants
User description
Closes #88
📑 Description
✅ Checks
☢️ Does this introduce a breaking change?
ℹ Additional Information
Summary by Sourcery
Convert cache management APIs and repository implementations to asynchronous patterns, adding cancellation support, timeouts, and enhanced logging throughout the caching layers, and update dependent tests and SmtpMailer to use the new async methods.
New Features:
Enhancements:
Tests:
Note
I'm currently writing a description for your pull request. I should be done shortly (<1 minute). Please don't edit the description field until I'm finished, or we may overwrite each other. If I find nothing to write about, I'll delete this message.
Description
Changes walkthrough 📝
CacheManager.cs
Migrate CacheManager to Asynchronous OperationsSrc/CrispyWaffle/Cache/CacheManager.cs
CouchDBCacheRepository.cs
Enhance CouchDBCacheRepository with Async SupportSrc/CrispyWaffle.CouchDB/Cache/CouchDBCacheRepository.cs
RedisCacheRepository.cs
Refactor RedisCacheRepository for Asynchronous OperationsSrc/CrispyWaffle.Redis/Cache/RedisCacheRepository.cs
MemoryCacheRepository.cs
Update MemoryCacheRepository for Async MethodsSrc/CrispyWaffle/Cache/MemoryCacheRepository.cs
ICacheRepository.cs
Modify ICacheRepository for Async Method SupportSrc/CrispyWaffle/Cache/ICacheRepository.cs
CacheManagerTests.cs
Refactor CacheManager Tests for Asynchronous MethodsTests/CrispyWaffle.Tests/Cache/CacheManagerTests.cs
MemoryCacheRepositoryTests.cs
Update MemoryCacheRepository Tests for Async SupportTests/CrispyWaffle.Tests/Cache/MemoryCacheRepositoryTests.cs
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests